-
Notifications
You must be signed in to change notification settings - Fork 73
Fix preloading of files from code fixes on Windows #377
Fix preloading of files from code fixes on Windows #377
Conversation
Codecov Report
@@ Coverage Diff @@
## master #377 +/- ##
==========================================
- Coverage 74.63% 74.61% -0.02%
==========================================
Files 15 15
Lines 1762 1765 +3
Branches 332 333 +1
==========================================
+ Hits 1315 1317 +2
Misses 296 296
- Partials 151 152 +1
Continue to review full report at Codecov.
|
src/typescript-service.ts
Outdated
@@ -1245,7 +1245,9 @@ export class TypeScriptService { | |||
|
|||
return this.projectManager.ensureOwnFiles(span) | |||
.concat(Observable.defer(() => { | |||
const configuration = this.projectManager.getConfiguration(fileTextChanges[0].fileName) | |||
// Configuration lookup uses Windows paths, FileTextChanges uses unix paths. Convert to backslashes. | |||
const firstChangedFile = uri2path(path2uri(fileTextChanges[0].fileName)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the purpose of this is to convert to backslashes, why not simply do filename.replace(/\//g, '\\')
? Converting to a URI and back seems like a hack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the intention is to only do it if it's a path with a drive letter, I'd still prefer to leave the URI functions out of this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's not the cleanest looking solution :) I'll extract the needed logic from uri2path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
For #370, where getConfiguration now takes windows paths.
The paths from typescript need to be converted from forward slashes back to windows paths.
Small fix, do you want a test or discuss normalised paths?